feat: 배달팟 삭제 API 구현#92
Conversation
📝 WalkthroughWalkthroughAdds delivery-party cancellation through a DELETE endpoint. The service validates existence and creator ownership, the domain transitions recruiting parties to ChangesDelivery party cancellation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant DeliveryPartyController
participant DeliveryPartyService
participant deliveryPartyRepository
participant DeliveryParty
Client->>DeliveryPartyController: DELETE /{partyId} with currentUserId
DeliveryPartyController->>DeliveryPartyService: deleteDeliveryParty(partyId, currentUserId)
DeliveryPartyService->>deliveryPartyRepository: findById(partyId)
deliveryPartyRepository-->>DeliveryPartyService: DeliveryParty
DeliveryPartyService->>DeliveryParty: cancel()
DeliveryParty->>DeliveryParty: Set status to CANCELED
DeliveryPartyService->>deliveryPartyRepository: save(DeliveryParty)
DeliveryPartyService-->>DeliveryPartyController: partyId
DeliveryPartyController-->>Client: 200 OK with partyId
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/test/java/com/leets/tdd/party/service/DeliveryPartyServiceTest.java (1)
150-190: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the actual soft-cancellation contract.
This passes if the service merely returns
partyId. AssertCANCELEDand repository persistence; add failure tests for missing party, non-owner, and non-RECRUITINGstates promised by this PR.Proposed test assertions
// then +verify(deliveryPartyRepository).save(deliveryParty); +assertThat(deliveryParty.getStatus()).isEqualTo(PartyStatus.CANCELED); assertThat(result) .isEqualTo(1L);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/leets/tdd/party/service/DeliveryPartyServiceTest.java` around lines 150 - 190, Update 배달팟_삭제_성공 to assert that deleteDeliveryParty changes the DeliveryParty status to CANCELED and persists the updated entity through deliveryPartyRepository, not only that it returns the party ID. Add failure tests covering a missing party, a non-owner requester, and parties whose status is not RECRUITING, asserting the expected exceptions and that no persistence occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/leets/tdd/party/controller/DeliveryPartyController.java`:
- Around line 90-104: The deleteDeliveryParty endpoint in
DeliveryPartyController is missing its Swagger documentation; add an operation
description documenting owner-only cancellation, the transition to CANCELED, the
successful deleted-party response, and 400, 403, and 404 error responses.
- Around line 91-100: Secure the delivery-party delete and matching update
endpoints by removing their route from SecurityConfig.PUBLIC_ENDPOINTS, and
change their `@AuthenticationPrincipal` parameters to the configured UserPrincipal
type, extracting the user ID as needed by deliveryPartyService. Add a failing
`@WebMvcTest` that verifies unauthenticated requests to the endpoint are rejected.
---
Nitpick comments:
In `@src/test/java/com/leets/tdd/party/service/DeliveryPartyServiceTest.java`:
- Around line 150-190: Update 배달팟_삭제_성공 to assert that deleteDeliveryParty
changes the DeliveryParty status to CANCELED and persists the updated entity
through deliveryPartyRepository, not only that it returns the party ID. Add
failure tests covering a missing party, a non-owner requester, and parties whose
status is not RECRUITING, asserting the expected exceptions and that no
persistence occurs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 288ec29d-4828-4e4c-870c-e7263293d008
📒 Files selected for processing (5)
src/main/java/com/leets/tdd/party/controller/DeliveryPartyController.javasrc/main/java/com/leets/tdd/party/domain/DeliveryParty.javasrc/main/java/com/leets/tdd/party/exception/PartyErrorCode.javasrc/main/java/com/leets/tdd/party/service/DeliveryPartyService.javasrc/test/java/com/leets/tdd/party/service/DeliveryPartyServiceTest.java
| // 배달팟 삭제(취소) API | ||
| @DeleteMapping("/{partyId}") | ||
| public ResponseEntity<Long> deleteDeliveryParty( | ||
| @PathVariable Long partyId, | ||
| @AuthenticationPrincipal Long currentUserId | ||
| ) { | ||
|
|
||
| Long deletedPartyId = | ||
| deliveryPartyService.deleteDeliveryParty( | ||
| partyId, | ||
| currentUserId | ||
| ); | ||
|
|
||
| return ResponseEntity.ok(deletedPartyId); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Complete the Swagger contract for this endpoint.
The PR explicitly leaves Swagger unfinished, so consumers will not see the owner-only cancellation behavior, CANCELED transition, response, or 400/403/404 errors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/leets/tdd/party/controller/DeliveryPartyController.java`
around lines 90 - 104, The deleteDeliveryParty endpoint in
DeliveryPartyController is missing its Swagger documentation; add an operation
description documenting owner-only cancellation, the transition to CANCELED, the
successful deleted-party response, and 400, 403, and 404 error responses.
| @DeleteMapping("/{partyId}") | ||
| public ResponseEntity<Long> deleteDeliveryParty( | ||
| @PathVariable Long partyId, | ||
| @AuthenticationPrincipal Long currentUserId | ||
| ) { | ||
|
|
||
| Long deletedPartyId = | ||
| deliveryPartyService.deleteDeliveryParty( | ||
| partyId, | ||
| currentUserId |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -HI -t f '.*Security.*\.java' src/main/java src/test/java
rg -n -C3 '`@AuthenticationPrincipal`|authorizeHttpRequests|requestMatchers|SecurityFilterChain|setPrincipal|Authentication' \
src/main/java src/test/javaRepository: Leets-Official/TDD-BE
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -HI -t f '.*(UserPrincipal|DeliveryPartyController|DeliveryPartyControllerTest|SecurityConfig).java' src/main/java src/test/java
echo "== DeliveryPartyController relevant section =="
cat -n src/main/java/com/leets/tdd/party/controller/DeliveryPartyController.java | sed -n '1,140p'
echo "== SecurityConfig =="
cat -n src/main/java/com/leets/tdd/global/config/SecurityConfig.java
echo "== JwtAuthenticationFilter relevant section =="
cat -n src/main/java/com/leets/tdd/global/config/JwtAuthenticationFilter.java | sed -n '1,115p'
echo "== UserPrincipal def/classpath search =="
rg -n -C5 'class UserPrincipal|record UserPrincipal|UserPrincipal' src/main/java src/test/java | head -n 200
echo "== DeliveryPartyControllerTest files =="
fd -HI -t f 'DeliveryPartyControllerTest.java' src/test/java -x sh -c 'echo "--- $1"; sed -n "1,220p" "$1"' sh {}Repository: Leets-Official/TDD-BE
Length of output: 35002
🌐 Web query:
Spring Security @AuthenticationPrincipal Long principal needs SecurityContext AuthenticationPrincipalArgumentResolver converter Long
💡 Result:
In Spring Security, the @AuthenticationPrincipal annotation relies on the AuthenticationPrincipalArgumentResolver to resolve the principal from the SecurityContext to a method argument [1][2]. When attempting to use @AuthenticationPrincipal directly with a Long or primitive long, you may encounter issues because the resolver checks if the principal is assignable to the requested parameter type [3][4]. Important technical constraints: 1. Primitive Types Not Supported: You cannot use primitive types (e.g., long) as arguments for @AuthenticationPrincipal, as these are not considered assignable from the object stored in the SecurityContext, which is typically a wrapper type or custom object [3][4]. Even if the underlying principal is a Long, using a primitive long parameter often causes errors or results in a null value [4]. 2. Type Mismatch: If the principal stored in the SecurityContext is not directly assignable to the type requested in your controller (e.g., you are trying to inject a Long but the principal is a custom UserDetails object), the resolver will return null by default [1][5]. If you set errorOnInvalidType = true in the annotation, it will throw a ClassCastException [1][6]. Recommended Approaches: If your principal object is a custom user class and you simply need to extract a Long ID: 1. Use a SpEL Expression: You can use the expression attribute of @AuthenticationPrincipal to navigate to the desired field [6][2]. If your principal has a getId method, you can use: @AuthenticationPrincipal(expression = "id") Long userId 2. Use a Custom Meta-Annotation or Argument Resolver: If you need more complex conversion logic that SpEL cannot handle, you should create a custom argument resolver. You can implement the HandlerMethodArgumentResolver interface to inspect the security context and perform manual conversion of the principal to your desired Long type. 3. Controller Logic: Alternatively, inject the full principal object (e.g., @AuthenticationPrincipal CustomUser user) and retrieve the ID inside your method body using user.getId. This is often the cleanest and most readable approach [2]. If you are encountering specific errors with Long conversion, ensure that the object stored in your Authentication principal is indeed the type you expect (e.g., verify that principal is not a UserDetails wrapper when you are expecting a raw Long) [1][7].
Citations:
- 1: https://docs.spring.io/spring-security/reference/api/java/org/springframework/security/web/method/annotation/AuthenticationPrincipalArgumentResolver.html
- 2: https://docs.spring.io/spring-security/reference/servlet/integrations/mvc.html
- 3: Exception using @AuthenticationPrincipal with primitive parameters spring-projects/spring-security#4245
- 4: AuthenticationPrincipal argument type cannot be primitive spring-projects/spring-security#10172
- 5: https://github.com/spring-projects/spring-security/blob/master/web/src/main/java/org/springframework/security/web/bind/support/AuthenticationPrincipalArgumentResolver.java
- 6: https://docs.spring.io/spring-security/reference/api/java/org/springframework/security/core/annotation/AuthenticationPrincipal.html
- 7: Spring Session integration do not supports primitive type as the principal spring-projects/spring-security#6571
Require authentication and accept the configured principal type.
/api/v1/delivery-parties/** is permitted to all requests in SecurityConfig, so this delete endpoint and the matching update endpoint bypass the JWT checker. If a token is present, JwtAuthenticationFilter stores a UserPrincipal, not a Long, so @AuthenticationPrincipal Long will not resolve under the current config. Remove this from PUBLIC_ENDPOINTS unless these methods are genuinely public, and/or change these parameters to @AuthenticationPrincipal UserPrincipal. Add a failing @WebMvcTest for the unauthenticated endpoint.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/leets/tdd/party/controller/DeliveryPartyController.java`
around lines 91 - 100, Secure the delivery-party delete and matching update
endpoints by removing their route from SecurityConfig.PUBLIC_ENDPOINTS, and
change their `@AuthenticationPrincipal` parameters to the configured UserPrincipal
type, extracting the user ID as needed by deliveryPartyService. Add a failing
`@WebMvcTest` that verifies unauthenticated requests to the endpoint are rejected.
작업 내용
변경 사항
리뷰 포인트
체크리스트
Summary by CodeRabbit
New Features
Bug Fixes
Tests